-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Chat onStartReached
loader fix
#33343
Chat onStartReached
loader fix
#33343
Conversation
@getusha, by chance, do you remember why you added this style? |
@perunt i am not sure if the style was added later but we experienced an issue on iOS and was resolved by adding it. 96151dc BUG8: money request view position is wrong when no messages #24482 (comment) |
hmm, thanks for the info! |
@thienlnam Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
@aimane-chnaif Since you reviewed the original PR do you want to be the C+ on this issue? |
@thienlnam sure! |
Please add tests step in Tests / QA section, which is clearly reproducible on main but not on this PR. |
ok, so the actual changes in the PR are:
|
Reviewer Checklist
Screenshots/VideosAndroid: Nativeandroid.movAndroid: mWeb Chromemchrome.moviOS: Nativeios.moviOS: mWeb Safarimsafari.movMacOS: Chrome / Safariweb.movMacOS: Desktopdesktop.mov |
@thienlnam can we merge it? |
@perunt please double confirm #33343 (comment) and do #33343 (comment). |
right, it always stretches the container even if we have a few items inside.
I didn't add any paddings in my PR. What do you mean? test steps from nested PRs
|
Before: After: So |
…idirectional-list-loader-mobile-fix
Correct. |
@perunt When you get a chance, please handle #33343 (comment) |
I added them here #33343 (comment) |
You may have to merge main to fix the typescript checks, I'm not sure why it's erroring on your build but I asked here |
…idirectional-list-loader-mobile-fix
Thanks for pointing it up |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/thienlnam in version: 1.4.23-0 🚀
|
🚀 Deployed to production by https://github.com/thienlnam in version: 1.4.23-4 🚀
|
@thienlnam can you please create GH for payment? Thanks |
bump ^ |
The issue is that the loader doesn't have space to appear. When we pull the page, the loader is visible momentarily, but it's immediately hidden by the list. This happens because we removed the
containerStyles
of the list for mobile. This change was made to fix a bug that arose after upgrading React Native WebOriginal PR where this style comes from PR.
Details
Fixed Issues
$
PROPOSAL:
Tests
Please check tests from both linked PRs
Test 1:
Test 2. Money request view position is wrong when there are no messages
follow this link: Upgrade to react-native-web v0.19.9 #24482 (comment). Open a video of the bug with the number 8 and follow this video's instructions
Verify that no errors appear in the JS console
Offline tests
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop